Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs(server-renderer) fix typo in renderToWebStream error, and remove Cloudflare Worker reference #4249

Merged
merged 2 commits into from
Aug 6, 2021

Conversation

Cherry
Copy link
Contributor

@Cherry Cherry commented Aug 4, 2021

Fixing the typo is pretty self explanatory, but the Cloudflare Workers doc change needs a little more explanation:

Unfortunately, Cloudflare Workers does not expose the ReadableStream constructor, which means it's not as simple as return new Response(renderToWebStream(app)) here.

There are ways to get this to work in Cloudflare Workers via ponyfill-ing ReadableStream, and then manually writing back to a TransformStream which is available in their environment, and ends up looking something like this:

...
import {ReadableStream as polyReadableStream} from 'web-streams-polyfill/dist/ponyfill.es2018.mjs';

async function pipeReaderToWriter(reader, writer){
	const encoder = new TextEncoder();
	for(;;){
		const {value, done} = await reader.read();
		const encoded = encoder.encode(value);
		await writer.write(encoded);
		if(done){
			break;
		}
	}
	writer.close();
}
...
...
const {readable, writable} = new TransformStream();
const render = renderToWebStream(app, {}, polyReadableStream);
pipeReaderToWriter(render.getReader(), writable.getWriter());
return new Response(readable);
...

More discussion can be found in #4243.

I figured that this complexity is very Cloudflare Workers specific, so perhaps wouldn't make sense to include in the README here. However I think it's a good idea to remove the CF Workers reference here, so others aren't confused about this not working in future. If it makes sense, I'd be happy to contribute some docs somewhere (where?) about this, or extend this README if you feel it makes the most sense here. Let me know.

@posva posva added the 🧹 p1-chore Priority 1: this doesn't change code behavior. label Aug 4, 2021
@yyx990803 yyx990803 merged commit 17cc4e1 into vuejs:master Aug 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧹 p1-chore Priority 1: this doesn't change code behavior.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants